Skip to content

Conversation

@hassoncs
Copy link

@hassoncs hassoncs commented Oct 10, 2025

Adds a runInBackground parameter to the execute_command tool. This allows commands, such as dev servers or monitoring processes, to be executed in the background without requiring user interaction or blocking further execution.

We've had a bunch of users request this feature:
Kilo-Org/kilocode#2917
Kilo-Org/kilocode#2196

The change includes updates to the tool definition, implementation, and corresponding tests.

2025-10-10 11 30 37


Important

Adds runInBackground parameter to execute_command tool for background command execution, with tests and documentation updates.

  • Behavior:
    • Adds runInBackground parameter to execute_command tool in executeCommandTool.ts to allow commands to run in the background.
    • Defaults runInBackground to false if not specified.
  • Tests:
    • Adds tests for runInBackground in executeCommandTimeout.integration.spec.ts and executeCommandTool.spec.ts to verify background execution behavior.
  • Documentation:
    • Updates execute-command.ts to include runInBackground parameter in the tool description and usage examples.

This description was created by Ellipsis for 640466c. You can customize this summary. It will automatically update as commits are pushed.

@hassoncs hassoncs requested review from cte, jr and mrubens as code owners October 10, 2025 18:33
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Oct 10, 2025
Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own code is like debugging in a mirror - twice the bugs, half the excuses.

completed = true
process.continue()
return
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: Logic flaw in runInBackground implementation.

The current implementation has several issues:

  1. Only triggers on output: This code is inside the onLine callback, so it only executes when the command produces output. Commands that don't produce immediate output won't auto-continue.

  2. Premature completion flag: Setting completed = true here happens before the command actually completes (which is signaled by onCompleted callback). This could interfere with the exit code handling logic at lines 334-361.

  3. Variable shadowing confusion: Line 181 declares let runInBackground = runInBackgroundRequested, but this variable serves a different purpose - it tracks whether the user chose to run in background via the UI prompt (line 212). The current implementation conflates these two concepts.

Suggested fix:

The auto-continue logic should be placed where the user interaction prompt happens (around line 211), not in the onLine callback:

// kilocode_change start
if (runInBackgroundRequested) {
    // Auto-continue without user interaction
    runInBackground = true
    process.continue()
    return
}
// kilocode_change end

try {
    const { response, text, images } = await task.ask("command_output", "")
    runInBackground = true
    // ... rest of existing logic
}

This ensures:

  • Auto-continue happens regardless of output
  • The completed flag is only set when the command actually completes
  • The two runInBackground concepts remain separate and clear

@hassoncs hassoncs force-pushed the exec-command-background-param branch from 640466c to b8c112e Compare October 10, 2025 18:40
Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own code is like debugging in a mirror - twice the bugs, half the excuses.

expect(mockTerminal.runCommand).toHaveBeenCalledWith("npm run dev", expect.any(Object))
expect(mockProcess.continue).toHaveBeenCalled()

// The task.ask method should not be called since runInBackground skips user interaction
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Test doesn't verify the key behavior. The comment says "task.ask method should not be called" but there's no assertion to verify this. Add an assertion to ensure task.ask is never called when runInBackground is true:

Suggested change
// The task.ask method should not be called since runInBackground skips user interaction
// The task.ask method should not be called since runInBackground skips user interaction
expect(mockTask.ask).not.toHaveBeenCalled()
})

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Oct 10, 2025
@hassoncs hassoncs force-pushed the exec-command-background-param branch 2 times, most recently from ad840b7 to 8788338 Compare October 10, 2025 18:51
Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found - all concerns already addressed in existing comments.

Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own code is like debugging in a mirror - twice the bugs, half the excuses.

No new issues found - all concerns already addressed in existing comments.

@hassoncs hassoncs force-pushed the exec-command-background-param branch from 8788338 to 01183cd Compare October 10, 2025 19:04
Copy link

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own code again? The existential dread of recursive self-analysis intensifies.

@hassoncs hassoncs force-pushed the exec-command-background-param branch 2 times, most recently from ad01db6 to ee05573 Compare October 15, 2025 20:23
Comment on lines 203 to 209
// kilocode_change start- If run_in_backgroundRequested, automatically continue the process
if (runInBackgroundRequested && !backgroundProcessStarted) {
backgroundProcessStarted = true
process.continue()
return
}
// kilocode_change end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-continue only triggers when command produces output. Commands that don't produce immediate output (or produce no output) won't auto-continue, defeating the purpose of runInBackground. For commands like npm run dev that may not output immediately, users will still see the interactive prompt.

The auto-continue should happen immediately when runInBackgroundRequested is true, not wait for output. Consider moving this logic before the try-catch at line 211 to ensure it runs regardless of output timing.

@hassoncs
Copy link
Author

hassoncs commented Oct 15, 2025

I updated this after discussing this with @hannesrudolph. I added more code in order to help the agent manage the running terminal processes. I couldn't think of a better way to do this other than adding a new tool, terminal_kill :( but I'm open to suggestions here~

I thought about maybe adding a new parameter to the execute_command tool to allow it to send a <signal>SIGINT</signal>, but that didn't seem very clean either. So, this solution seems to work, but I don't really love the fact that we had to add a tool :-/.

At first, I tried capturing the PIDs of the processes, but it's difficult to get the PID of the built-in VS Code terminal commands. You end up getting the shell process ID, which cannot be killed using the regular kill command, which is what the agent tries to do if you don't give it a new tool. So by giving it a tool, we can kill the process similar to the kill button that exists in the UI today. Essentially, this PR gives the agent the ability to click its own buttons in order to continue processes in the background and to kill things later.

Test plan:
I confirmed that the agent can run multiple long-running terminal commands, kill them arbitrarily, and still have visibility into running terminals when dispatching new tasks in orchestrator mode.

Testing prompt:

# Terminal Control Feature Testing Prompt

You are testing a new terminal control feature in Kilo Code that allows running commands in the background and managing them. Your goal is to thoroughly test all aspects of this functionality and report any bugs you encounter.

## Features to Test:

### 1. Background Command Execution
Test the new `run_in_background` parameter for the `execute_command` tool. When you want to test this, use commands like:
- `sleep 30` (for a simple long-running command)
- `ping google.com` (for continuous output)
- `npm run dev` (for development servers)
- `python -m http.server 8000` (for web servers)

## Test Scenarios:

### Scenario 1: Basic Background Execution
1. Start a long-running command in the background using `sleep 30`
2. Verify the command starts without waiting for user interaction
3. Verify you receive a terminal ID in the response
4. Confirm the process is actually running

### Scenario 2: Multiple Background Processes
1. Start 2-3 different long-running commands in the background:
   - `sleep 60`
   - `ping -c 100 google.com`
   - `python -m http.server 8001`
2. Verify each gets a unique terminal ID
3. Verify all processes run simultaneously
4. Check that terminal IDs are properly tracked

### Scenario 3: Terminal Process Management
1. Start a background process with `sleep 45`
2. Note the terminal ID from the response
3. Use `terminal_ctrl` with action "kill" to terminate the process
4. Verify the process is actually killed
5. Verify appropriate success/error messages

### Scenario 4: Error Handling
Test these error conditions:
1. Try to kill a non-existent terminal ID (use ID 999)
2. Try to kill a terminal with no running process
3. Use invalid action in `terminal_kill` (try "pause" instead of "kill")
4. Use `terminal_kill` without required parameters

### Scenario 5: Integration Testing
1. Start a development server in the background: `python -m http.server 8002`
2. Run other commands while the server is running (like `ls`, `pwd`)
3. Kill the background server using terminal_kill
4. Verify the server actually stopped

## Expected Behaviors:

1. **Background commands should:**
   - Start immediately without user approval prompts for continuation
   - Return a terminal ID for reference
   - Continue running while other commands execute
   - Provide status updates about the running process

2. **Terminal kill should:**
   - Successfully kill processes using terminal IDs
   - Provide clear success/failure messages
   - Handle non-existent terminal IDs gracefully

3. **Error handling should:**
   - Provide helpful error messages for invalid parameters
   - List available terminal IDs when referencing non-existent ones
   - Handle edge cases gracefully

@hassoncs hassoncs force-pushed the exec-command-background-param branch from ee05573 to 7299b67 Compare October 15, 2025 20:27

if (!targetTerminal.busy && !targetTerminal.process) {
return `Terminal ${terminalId} is not running any process.`
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent null checking for process. The condition at line 74 checks !targetTerminal.busy && !targetTerminal.process, which returns early when process is null. However, at line 85 in the ExecaTerminal branch, the code calls targetTerminal.process.abort() without verifying process exists. This creates a potential null pointer exception if targetTerminal.busy is true but targetTerminal.process is null.

The early return condition should be if (!targetTerminal.process) to ensure process exists before attempting to use it in either branch.

- Add new `terminal_kill` tool for terminating terminal processes
- Enhance `execute_command` tool with `run_in_background` parameter for long-running commands
- Update tool descriptions, handlers, and tests accordingly
@hassoncs hassoncs force-pushed the exec-command-background-param branch from 7299b67 to 9dea0b5 Compare October 16, 2025 04:59
Add terminalId validation

Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
@hassoncs
Copy link
Author

Sorry for the delay- I'm going to close this PR while I iterate so I don't cause unnecessary noise and my the Roo bot work too hard. It'll be back soon! :)

@hassoncs hassoncs closed this Oct 17, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Oct 17, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants